-
-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix linuxboot builds #845
base: master
Are you sure you want to change the base?
Fix linuxboot builds #845
Conversation
@NHellFire Ci fails because #842 not being merged. This will need to be rebased ton top of master once merged in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait, why is coreboot being included as a module for a Linuxboot build anyway? Seems like that's the issue that needs to be fixed
@MrChromebox I like the fix. Rebasing on master with linixbootboarf added in CI should show the fix working. |
@NHellFire can you add the board in CircleCI and rebase on master? |
…linuxboot#841) Signed-off-by: Nathan Rennie-Waldock <[email protected]>
6bb5b59
to
7e2d39d
Compare
Signed-off-by: Nathan Rennie-Waldock <[email protected]>
7e2d39d
to
c20c890
Compare
I've rebased (and fixed showing the error). |
@NHellFire So if I understand well, this will work once linuxboot has your PR merged upstream, while linuxboot will not offer reproducibility since not bound to particular commit ID as other modules. Actually, it seems that linuxboot is the only module still not being bound to a commit id EDIT: confirmed:
Consequence, it will be impossible to reproduce Head's linuxboot based ROM based on a signle Head's commit ID in the future. Might need another issue and PR, or added to this PR, since linuxboot cannot be build for more then a month now (coreboot 4.8.1 version specification inside of coreboot module), and CircleCI's builds are not built since a while. It seems that linuxboot builds are not often built for the following platforms:
|
Signed-off-by: Nathan Rennie-Waldock <[email protected]>
CI should pass now. Local build passed with gcc 9.3.0 and binutils 2.34 (Ubuntu 20.04.1). |
@NHellFire not sure about the hack implemented in the linuxboot module to apply patches outside of the root Makefile logic. if we look at musl-cross-make module, being dependend on a specific commit for reproducible builds in the future on a specific Heads commit being rebuild, the following module specifies what to download as opposed to downloading master which cannot be replicated in the future if master changes. As a result for that specific musl-cross-make git version, no patch are currently applied (no patch exists for that specific module-hash), since no patches relative to that specific commit id exits (Duh. there is a past artifact for musl-cross lying around, will fix that). Does that make sense? In the present case, linuxboot should be fixed to commit id linuxboot/linuxboot@10be0dc into module, and your linuxboot-ed2k patches should be applyable directly inside of the decompressed archive for that specific commit ID, where the patch would be in a file patches/linuxboot-10be0dc2652e1fc8c11d419ca2c4ff93920d4165.patch or in seperate files under patches/linuxboot-https://github.com/linuxboot-10be0dc2652e1fc8c11d419ca2c4ff93920d4165/1.patch etc following root Makefile patch application rule. |
I'll see if I can get to this in the next few days. I've been away from
home and a bit busy.
…On Fri, 16 Oct 2020, 21:00 tlaurion, ***@***.***> wrote:
@NHellFire <https://github.com/NHellFire> ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#845 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIYA243XRIHP7XTQVGP633SLCQ5NANCNFSM4R3GFE3A>
.
|
@NHellFire please tag me for followups |
> Running edk2 build for OvmfPkgX64 > Usage: build.exe [options] [all|fds|genc|genmake|clean|cleanall|cleanlib|modules|libraries|run] > > build.exe: error: option -n: invalid integer value: 'PUS' Signed-off-by: Nathan Rennie-Waldock <[email protected]>
* add linuxboot-edk2 module (if not building latest git) * patch linuxboot-edk2 Makefile to not conflict with ours * update linuxboot build dir in circleci and boards/winterfell Signed-off-by: Nathan Rennie-Waldock <[email protected]>
…gcc 8 Signed-off-by: Nathan Rennie-Waldock <[email protected]>
f48a5db
to
8a1eb75
Compare
@tlaurion Can you check the latest revision? |
@@ -52,7 +52,7 @@ dxe_offset := 860000 | |||
dxe_size := 6a0000 | |||
flash-dxe: $(build)/$(BOARD)/linuxboot.rom | |||
( echo u$(dxe_offset) $(dxe_size) ; \ | |||
pv $(build)/linuxboot-git/build/$(BOARD)/dxe.vol \ | |||
pv $(build)/linuxboot-b5376a441e8e85cbf722e943bb8294958e87c784/build/$(BOARD)/dxe.vol \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I get what you are doing here.
You can get the build directory by reusing Makefile included module.
In this case: $(linuxboot_base_dir)
command: | | ||
make \ | ||
BOARD=qemu-linuxboot \ | ||
`/bin/pwd`/build/linuxboot-b5376a441e8e85cbf722e943bb8294958e87c784/build/qemu/.configured \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That will break in future linuxboot version bump. Why static?
$(linuxboot_base_dir)/build/qemu/.configured
?
@@ -1,5 +1,6 @@ | |||
modules-$(CONFIG_COREBOOT) += coreboot | |||
|
|||
ifeq "$(CONFIG_COREBOOT)" "y" | |||
ifeq "$(CONFIG_COREBOOT_VERSION)" "4.8.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation should shift
|
||
linuxboot-edk2_tar := linuxboot-edk2-$(linuxboot-edk2_version).tar.gz | ||
linuxboot-edk2_url := https://github.com/linuxboot/edk2/archive/$(linuxboot-edk2_version).tar.gz | ||
linuxboot-edk2_dir := $(linuxboot_base_dir)/edk2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so that is the env variable to reuse $linuxboot-edk2_dir
@NHellFire I see that requests for change in this PR are still valid? |
@@ -21,7 +27,7 @@ linuxboot_configure := \ | |||
if [ "$(linuxboot_board)" = "qemu" ]; then \ | |||
echo >&2 "Pre-building edk2 OVMF" ; \ | |||
( cd $(build)/$(linuxboot_base_dir)/edk2/OvmfPkg ; \ | |||
./build.sh -n $$CPUS \ | |||
./build.sh -n $(CPUS) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tlaurion What do you say to making this change a separate PR and merging it in? I was about to make one addressing this before seeing this change in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@synackd please do. Same should be made under coreboot module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested it? The idea here is to pass CPUS to command line
make BOARD= x230 CPUS=2
There is no such invocation as CPUS as a command.
@synackd?
EDIT: nevermind! Tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes can be retracted since they are already merged as of #940.
@NHellFire @synackd ping into cleaning this PR? |
@NHellFire patches are under patches/module-version and applied automatically, if modules respect module statements. |
Linked with #971 (comment) |
Tested builds of qemu-linuxboot and qemu-coreboot, both succeeded with gcc 7. With gcc 8+, linuxboot will fail on edk2 (linuxboot/linuxboot#43).